Skip to content

docs(skills): objectstack-api teaches the recycle bin a by-id delete keeps, not a hard delete - #30994

Merged
os-zhuang merged 4 commits into
mainfrom
claude/project-thread-zyyxri
Nov 6, 2026
Merged

os-zhuang merged 4 commits into
mainfrom
claude/project-thread-zyyxri

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Irene · project thread

Refs #30847

Clause-②: no
Metadata-protocol: none

The maintainer's standing order in this project (2026-11-03) waives the contract-review model tier that dispatch-gates mandates for skills/** (clause ①); this PR was developed below that tier, and the dispatching seat runs the contract review.

维护者速读

  • 改了什么:skills/objectstack-api/SKILL.md 三处:L48 路由表 DELETE 注释由 "hard delete" 改为 "business-object rows → recycle bin";派生动词说明 restore / purge 不派生的理由由"trash 面已退役"改为"回收站还没有 restore / purge 入口";陷阱第 2 条改写为:按 id 删除业务对象会把行保留在 sys_recycle_bin(默认 30 天),multi: true 删除、平台行、回收站上线前删除的行不在其中,目前没有任何 API 能列出或恢复它(sys_recycle_bin 为 apiEnabled: false),不要自造软删除。评测 evals/endpoints-auth-routes.json 第 5 例的期望同步改为回收站的说法。另加一条 changeset。
  • 为什么改:PR feat(objectql,platform-objects): keep a by-id delete's rows in the recycle bin (ADR-0148) #30780(ADR-0148)合入后,按 id 删除会把行快照进回收站,技能却仍教"硬删除、没有回收站",读技能的 agent 会自造软删除,或告诉用户删除的记录无法找回。
  • 风险与回滚:只改技能文本,运行时零改动;三段各自行数不变,文件 441 → 441 行,token 5141 → 5157(上限 6314 不动,不付账);回滚即 revert。
  • 先看:SKILL.md L48、L268-270、L417-420;评测第 5 例。
  • 范围:本 PR 只做 triage 审计要的"立即纠正"部分,所以写 Refs 不写 Fixes;restore / purge 入口、权限与 REST 路由的教学等 engine: snapshot deletes into the recycle bin, restore under original ids, purge, and the TTL reap (#3146 slice 3) #30237 的 restore 与 REST 入口 PR 落地后再补。
  • 需要你做的:Tier H(skills/**),需要受管审批人在 GitHub 上 Approve 一次;之后席位落地。

What changes

skills/objectstack-api/SKILL.md, three passages, each at its original line count:

Where Before After
L48, data route block DELETE … # Delete record (hard delete) DELETE … # Delete record (business-object rows → recycle bin)
L268-270, derived verbs "restore / purge never derive — the trash surface is retired." "restore / purge never derive: the recycle bin has no restore or purge door yet." (re-flowed inside its own three lines)
L417-420, pitfall 2 "ObjectStack DELETE is a hard delete — there is no recycle bin. For recoverability, use per-field trackHistory … or a lifecycle archive policy instead of custom soft-delete logic." "Treating DELETE as final or as restorable. A by-id delete keeps a business object's rows in the recycle bin (sys_recycle_bin, 30 days by default; not multi: true deletes, platform rows or rows deleted before the bin existed); nothing lists or restores it over the API yet. No soft-delete."

skills/objectstack-api/evals/endpoints-auth-routes.json, case 5 only (claim surface widened by the seat in rework round 1): expected_output no longer says restore / purge never derive "because DELETE is a hard delete; recoverability is per-field trackHistory or a lifecycle archive policy". It now says they never derive, a by-id delete of a business object already keeps its rows in the recycle bin (sys_recycle_bin), but nothing lists or restores them over the API yet, so there is no restore operation to add and no custom soft-delete to build. must_contain swaps trackHistory / lifecycle for sys_recycle_bin / recycle bin; must_not_contain keeps 'restore'.

.changeset/api-skill-delete-recycle-bin.md: '@objectstack/skills': patch, both declaration lines. No skip-changeset label: a skills/** diff ships in @objectstack/skills.

Each claim, read off main:

  • A by-id delete keeps the rows. packages/objectql/src/recycle-bin/capture.ts (PR feat(objectql,platform-objects): keep a by-id delete's rows in the recycle bin (ADR-0148) #30780): ObjectQL.delete's by-id branch (runByIdDelete, engine.ts ≈L9514) snapshots the cascade set and writes one sys_recycle_bin entry before the root row is removed. The REST DELETE /data/{object}/:id reaches that branch (a scalar where.id).
  • Which deletes are not in it. The same docblock, "Which deletes enter the bin (v1)": not a predicate delete (multi: true), not a platform object (sys_, cloud_, ai_, hasPlatformObjectPrefix), and a platform row the cascade reaches is left out of the snapshot. Rows deleted before PR feat(objectql,platform-objects): keep a by-id delete's rows in the recycle bin (ADR-0148) #30780 have no entry.
  • 30 days by default. packages/objectql/src/recycle-bin/retention.ts: host resolver, else OS_RECYCLE_BIN_RETENTION_DAYS, else RECYCLE_BIN_DEFAULT_RETENTION_DAYS (30), stamped as purge_after; sys-recycle-bin.object.ts reaps on that deadline (ttl).
  • Nothing lists or restores the bin over the API. sys-recycle-bin.object.ts declares apiEnabled: false, apiMethods: [], so no generic data route reaches the bin. allowRestoreOwnDeleted / manageRecycleBin in packages/spec/src/security/permission.zod.ts are [PLANNED] … Not read yet; API_METHOD_DERIVATION keeps restore / purge at flag: () => false; no engine restore or REST route exists. So the skill names neither permission nor any door (engine: snapshot deletes into the recycle bin, restore under original ids, purge, and the TTL reap (#3146 slice 3) #30237 is still open).
  • restore / purge still never derive and are still stripped from apiMethods with a FROM → TO warning (LEGACY_API_METHOD_GUIDANCE, object.zod.ts), so that half of the L267 sentence stands.

Dropped from the pitfall: the enable.trash history (PR #30844 had already cut it) and the pointer to trackHistory / lifecycle archive as the recoverability answer, which #30847 names as part of the wrong teaching.

Budget. skills/objectstack-api/SKILL.md: 441 → 441 lines, 20542 → 20646 bytes, 5141 → 5157 tokens (ceiling 6314, headroom 1157). The eval fixture keeps its line count (case 5's two lines change in place; measured, not ratcheted: 1498 → 1529 tokens). Net added lines in skills/**: 0. No ceiling row moves, so no deletion payment is owed.

Serial. PR #30844 (seat 2) edited the same pitfall. origin/main at 000a4c0d (with #30844) was merged into the branch, no rebase; the one conflict, pitfall 2, takes this PR's rewrite. Rework round 1 merged origin/main c77dd17f (clean) before its commit.

Why

#30847 (p2): since PR #30780 the skill contradicts shipped behaviour. An agent following it builds its own soft-delete, or tells a user a deleted record is gone when its snapshot sits in the bin. Triage audit 6679098621 asks for the "no recycle bin" correction now and the restore-door teaching once #30237's restore and REST-door PRs land; this PR is the first half, hence Refs.

Verification

Derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack against the three-file diff, each command run under timeout 900 with stdin closed after a full pnpm build, exit codes recorded first:

  • At 211f3a99 (first head, base 1b3e22f0): 45 derived, 45 run, 45 exit 0.
  • At 14487c50 (after merging origin/main 000a4c0d): 45 derived, 45 run, 45 exit 0.
  • At 9a449ed9 (the head, rework round 1, after merging origin/main c77dd17f): 45 derived, 45 run, 45 exit 0. dispatch-gates --ran: 45 accounted, 0 NOT-MEASURED, 0 UNRUN.
  • check-skills-token-ratchet: skills/objectstack-api/SKILL.md is 5157 tokens (ceiling 6314; headroom 1157).
  • node scripts/check-commit-card-trailers.mjs --range origin/main..HEAD: 4 commits, exit 0, before each push.

Ran lists: card-30847/gates-{211f3a99,14487c50,9a449ed9}.txt in the project files.

Ran list at 9a449ed
node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0
node scripts/check-adr-0087-registration.mjs --self-test :: exit 0
node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0
node scripts/check-closing-keyword-parity.mjs :: exit 0
node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0
node scripts/check-comment-mask-corpus.mjs :: exit 0
node scripts/check-doc-route-spelling.mjs --advisory :: exit 0
node scripts/check-doc-route-spelling.mjs --self-test :: exit 0
node scripts/check-dts-emitted.mjs --self-test :: exit 0
node scripts/check-empty-changeset.mjs --base origin/main :: exit 0
node scripts/check-empty-changeset.mjs --self-test :: exit 0
node scripts/check-skills-token-ratchet.mjs :: exit 0
node scripts/check-skills-token-ratchet.mjs --self-test :: exit 0
node scripts/ci/assert-turbo-remote-cache.mjs --self-test :: exit 0
node scripts/ci/plan-test-core-shards.mjs --self-test :: exit 0
node scripts/ci/prune-turbo-cache.mjs --self-test :: exit 0
node scripts/ci/snapshot-next.mjs --self-test :: exit 0
node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0
node scripts/release-pending-publish.mjs --self-test :: exit 0
pnpm --filter @objectstack/lint run check:doc-formula-expressions :: exit 0
pnpm --filter @objectstack/spec run check:skill-docs :: exit 0
pnpm check:agent-test-spelling :: exit 0
pnpm check:changeset-gate-self-tests :: exit 0
pnpm check:ci-filter-parity :: exit 0
pnpm check:closed-ceremony-text :: exit 0
pnpm check:corpus-claim-drift :: exit 0
pnpm check:cross-package-test-inputs :: exit 0
pnpm check:doc-authoring :: exit 0
pnpm check:driver-memory-census :: exit 0
pnpm check:dts-closure :: exit 0
pnpm check:dual-build-cjs-loads :: exit 0
pnpm check:gitlink-declared :: exit 0
pnpm check:lean-entry-closure :: exit 0
pnpm check:merge-driver :: exit 0
pnpm check:nul-bytes :: exit 0
pnpm check:pm-governed-merges :: exit 0
pnpm check:pm-skill-id-lint :: exit 0
pnpm check:published-files :: exit 0
pnpm check:refd-timer-probe :: exit 0
pnpm check:role-word :: exit 0
pnpm check:skill-compatibility :: exit 0
pnpm check:skill-frame-sync :: exit 0
pnpm check:skill-identifier-liveness :: exit 0
pnpm check:sourcemap-no-sources-content :: exit 0
pnpm check:watch-hint-literal :: exit 0

CI families (Lint & Repo Gates, Test Core, the rest) are the PR's check-runs, not re-run locally.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic.
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint is CI's run (no source file in the diff).
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec not touched.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.
  • The Before Submitting list in CONTRIBUTING.md is done.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J8VVAnsTJVz6nCb7n8NUJs

…keeps

The data-API table, the derived-verb note and the DELETE pitfall said a
delete is hard and there is no recycle bin. A by-id delete of a business
object now keeps its rows in sys_recycle_bin (30 days by default); multi
deletes, platform rows and rows deleted before the bin existed are not in
it, and no restore or purge door is exposed yet.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8VVAnsTJVz6nCb7n8NUJs
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 6, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:skills#3 · huangyiirene · session_01J8VVAj7CvjkGXvgXzd6RZ7

Contract-review-claim: session_01J8VVAj7CvjkGXvgXzd6RZ7 · head 14487c5 · 2026-11-06T16:08Z

The seat review runs at CONTRACT_REVIEW_TIER in an isolated subagent. No review seat (lanes/review.md) is seated.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:skills#3 · huangyiirene · session_01J8VVAj7CvjkGXvgXzd6RZ7

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 14487c5027728964e39e56ee0cec56bff9c03d2a
Local-runs: none
Scope: skills/objectstack-api/SKILL.md + skills/objectstack-api/evals/endpoints-auth-routes.json, .changeset/api-skill-delete-recycle-bin.md, packages/objectql/src/recycle-bin/capture.ts, packages/objectql/src/recycle-bin/retention.ts, packages/objectql/src/engine.ts (runByIdDelete), packages/metadata-protocol/src/protocol-data-writes.ts (deleteData), packages/platform-objects/src/system/sys-recycle-bin.object.ts, packages/spec/src/data/recycle-bin.zod.ts, packages/spec/src/data/api-derivation.ts, packages/spec/src/data/object.zod.ts (lifecycle class rules), packages/spec/src/security/permission.zod.ts, packages/spec/src/system/constants/platform-object-names.ts, packages/plugins/plugin-security/src/permission-evaluator.ts, docs/adr/0148-recycle-bin-separate-store-restore-under-original-ids-retention-in-days.md, packages/skills/scripts/sync-catalog.mjs, ui/scripts/check-skill-eval-tokens.mjs, ui/scripts/skill-bundle-root.mjs

Inputs: card #30847 (body, triage 6679098621, claim comment), PR #30994 (body, two-file list), the net diff at the head against origin/main 000a4c0d3 (the merge base IS origin/main, so the net diff is the two files and nothing else), and the head's check-runs.

Check-runs at render time: every completed non-skipped run is success (Build Core, Check Changeset, Check PR Size, Governed Surface Queue Guard, PR-body guards, Lint · ESLint / PM dispatch-gates / docs & code guards / repo & tooling gates / the three scoped lanes, Type Check · workspace / source gates / debt ledger, Test UI, Live E2E UI, Dogfood Regression Gate). Still in progress, not waited on: Test Core (1/8), Test Core (2/8), Type Check · consumer gates, Lint · CI, release & engine gates. No run has failed. Note for the reader of the green Lint · repo & tooling gates: check:skill-eval-tokens walks skills/objectui only (ui/scripts/skill-bundle-root.mjs, SKILL_BUNDLE), so no gate reads skills/objectstack-api/evals/** and a green says nothing about the finding in ④.

① Derived judgments

No accept-set changes. The diff touches no schema, no ApiMethod / ApiOperation enum, no API_METHOD_DERIVATION row (restore / purge stay flag: () => false), no stripLegacyApiMethods path, no permission key and no route. Every runtime accept set on main is unchanged.

The published-surface change is the text of the @objectstack/skills catalog. packages/skills/scripts/sync-catalog.mjs copies skills/** byte for byte into dist/skills, so SKILL.md and evals/** both ship. Each teaching change, judged:

  • L48, route block. DELETE … # Delete record (kept in the recycle bin). Right for a business object: the REST door's deleteData calls engine.delete(object, { where: { id } }) with no multi, which reaches runByIdDelete → openRecycleBinCapture(..., predicate = false). Over-broad for the same route on a platform object the data API serves (for example sys_business_unit, which is apiEnabled: true): recycleBinTakesObject refuses every sys_ / cloud_ / ai_ name, so that delete is still physical with no entry. → should-fix S1.
  • L271–272, derived-verb rule. "restore / purge never derive: the recycle bin has no restore or purge door yet." Right on main. API_METHOD_DERIVATION keeps both at flag: () => false. The docblock there says they "return as live derived verbs only if/when a real recycle bin ships". No engine restore, purge or REST door exists; DESTRUCTIVE_OPERATIONS denies both fail-closed.
  • L411–414, pitfall 2. Right on every clause it states (see ④).
  • Not changed, but implied by the card: evals/endpoints-auth-routes.json id 5 still grades the retired teaching. This is wrong; see ④ M1.

② Semver level

'@objectstack/skills': patch matches what the diff publishes. The diff corrects catalog text in a package whose files ship dist/skills/**. It adds no capability, removes no export and touches no schema, so patch is right and skip-changeset would have been wrong. The M1 fix lands in the same package and stays patch.

Clause-②: no is right: no published schema, packages/spec surface or governed spec path is in the diff. Metadata-protocol: none is right: nothing in packages/metadata-protocol or the protocol's wire shape moves. The PR body and the changeset carry the same three lines, and Check Changeset is green.

③ Boundary flags

  • Contract-review tier waived (PR body, claim). The dev ran below the tier that dispatch-gates mandates for skills/**, under the maintainer's standing order of 2026-11-03. This record is the review at CONTRACT_REVIEW_TIER, so the tier is served here. Nothing is left to escalate on this point.

  • Partial scope, Refs #30847. Right. The card's Wanted list (who may restore or purge, with which permission, which door) cannot be met while engine: snapshot deletes into the recycle bin, restore under original ids, purge, and the TTL reap (#3146 slice 3) #30237 is open (checked: open). The diff names no door, no grant key (allowRestoreOwnDeleted and manageRecycleBin are [PLANNED] … Not read yet in permission.zod.ts) and no route. The card must stay open with Blocked-by: #30237, as the claim says.

  • Serial with PR docs(skills): objectstack-api teaches the /meta/:type list's ?names= filter and its closed query set #30844. That PR edits the same pitfall. The branch merged origin/main 000a4c0d (with docs(skills): objectstack-api teaches the /meta/:type list's ?names= filter and its closed query set #30844), and the net diff against main is exactly the three hunks, so docs(skills): objectstack-api teaches the /meta/:type list's ?names= filter and its closed query set #30844's content on main is untouched. Resolved.

  • Dropped pointer to trackHistory / a lifecycle archive. As a rule this loses nothing that is still true:

    • lifecycle retention, TTL and archive are refused on class: 'record', the business-truth class (object.zod.ts, ADR-0057 §3.1). An archive is age-based hot-to-cold storage, never an undo of a user's delete.
    • trackHistory is a field-change audit trail and gates history, which SKILL.md L271 still teaches. It does not recover a deleted record.
    • The surviving instruction ("build no soft-delete") keeps the old rule's real content, which was "no custom soft-delete". It agrees with ADR-0148 D1, where the IsDeleted shape is explicitly not chosen.

    But the drop leaves the skill's own eval grading for the removed words. → ④ M1.

  • Escalation: the claim's file surface. The claim lists SKILL.md (three passages) plus .changeset/*.md, "stop on breach". M1's fix is in skills/objectstack-api/evals/endpoints-auth-routes.json, which is outside that surface. The dispatching seat must widen the surface, or rule the eval fix a separate change. Either way, the catalog must not ship the corrected SKILL.md alongside the uncorrected eval.

  • Tier H approval (skills/**) is still owed by a governed approver. That is outside this record.

  • PR body "Verification" (45 derived commands, exit 0). This is the dev's account. It is not re-run here (read-only); the head's check-runs above are the verdicts.

④ Design judgment

skills/objectstack-api/SKILL.md at the head, judged against main and against the bundle's eval scenario for this exact topic.

True on main, each clause of pitfall 2 checked against the code:

Clause Evidence on main
A by-id delete keeps the rows engine.ts runByIdDelete: capture → cascade → bin.write() → driver.delete
"a business object's" recycleBinTakesObject = !hasPlatformObjectPrefix (sys_, cloud_, ai_). A cascaded platform row is filtered out of the snapshot
"rows" The root plus its master-detail cascade go in one entry (cascadeMembersInOrder)
sys_recycle_bin RECYCLE_BIN_OBJECT
"30 days by default" resolveRecycleBinRetentionDays: host resolver, else OS_RECYCLE_BIN_RETENTION_DAYS, else RECYCLE_BIN_DEFAULT_RETENTION_DAYS = 30, stamped as purge_after and reaped by the ttl on sys_recycle_bin
"never multi: true deletes" openRecycleBinCapture returns undefined on predicate. The engine passes options?.multi === true, even on the by-id route the lifecycle reap takes
"rows deleted before the bin existed" ADR-0148 D4 Forward-only. The capture only writes at delete time
"Restore is not exposed yet" No engine restore, purge or empty path. No REST route. Grants [PLANNED]. restore / purge are in DESTRUCTIVE_OPERATIONS, so they are denied fail-closed
"build no soft-delete" ADR-0148 D1

No undelivered door is taught. No route, grant key, restore() call or Console page is named. The changeset summary says the same.

L271–272 inside the derived-verb table. It reads correctly. "Never derive" is the resolver's present answer (flag: () => false). "Yet" does not suggest the verbs become authorable: the next sentence, "Declaring any derived verb in apiMethods is stripped at parse with a FROM → TO warning", still governs both. ADR-0148 D5 keeps the retired apiMethods values restore / purge retired. Whether they ever return as derived verbs is #30237's call, and the sentence does not prejudge it. The sentence does not contradict the table above it: neither verb is a table row.

Findings

  • M1 (must-fix): the bundle's own eval for this scenario still grades the retired teaching, and the corrected skill now fails it. skills/objectstack-api/evals/endpoints-auth-routes.json id 5 has the prompt "…add … a restore operation so deleted invoices can be undeleted". Its expected output is "restore / purge never derive because DELETE is a hard delete; recoverability is per-field trackHistory or a lifecycle archive policy". It carries must_contain: ["apiMethods", "/batch", "trackHistory", "lifecycle"]. This is the card's defect, word for word, and it ships in @objectstack/skills (sync-catalog.mjs copies skills/** whole). An agent answering prompt 5 from the corrected SKILL.md would say the invoice rows are kept in sys_recycle_bin and that restore is not exposed yet. The eval marks that answer wrong in two ways: its expected output says the opposite, and lifecycle is now taught nowhere in skills/objectstack-api/**/*.md, so its must_contain token cannot be earned. The fix belongs in this PR:

    • Rewrite id 5's expected_output to the new teaching: derived verbs are stripped; restore / purge do not derive; by-id deletes are kept in sys_recycle_bin; restore is not exposed yet; no custom soft-delete.
    • Replace trackHistory / lifecycle in must_contain with tokens the bundle teaches, such as sys_recycle_bin.
    • Keep 'restore' in must_not_contain.

    No gate catches this: check:skill-eval-tokens walks only skills/objectui.

  • S1 (should-fix): L48's route comment is unconditional. The same DELETE /api/v1/data/{object}/:id serves sys_ / cloud_ / ai_ objects that have the data API on (for example sys_business_unit). Those deletes are physical and enter no bin. Qualify the comment in place, for example # Delete record (business objects: kept in the recycle bin). That stays on one line and costs no budget.

  • S2 (should-fix): naming sys_recycle_bin invites a read the data API refuses. sys_recycle_bin is apiEnabled: false, apiMethods: []: nothing reads it over REST until engine: snapshot deletes into the recycle bin, restore under original ids, purge, and the TTL reap (#3146 slice 3) #30237's doors land. An agent asked "show me deleted invoices" will try GET /api/v1/data/sys_recycle_bin. Widen "Restore is not exposed yet" to "Nothing lists or restores it over the API yet". That stays inside the passage's four lines.

Not findings, for the record: the bin exists only when PlatformObjectsPlugin registers sys_recycle_bin; a composition without it logs info and keeps nothing. A delete also drops secret columns from the snapshot (ADR-0148 D4). Both are below the resolution of a REST-surface pitfall, and neither makes a sentence false for the standard runtime.

Implemented-by: claude/project-thread-zyyxri
Reviewed-by: session_01J8VVAj7CvjkGXvgXzd6RZ7

VERDICT: FAIL

…cts and updates eval case 5

The DELETE route comment names the condition (business-object rows), the
pitfall says nothing lists or restores the bin over the API yet
(sys_recycle_bin is apiEnabled: false), and eval case 5 expects the
recycle-bin teaching instead of a hard delete with trackHistory or a
lifecycle archive.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8VVAnsTJVz6nCb7n8NUJs
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:skills#3 · huangyiirene · session_01J8VVAj7CvjkGXvgXzd6RZ7

Contract-review-claim: session_01J8VVAj7CvjkGXvgXzd6RZ7 · head 9a449ed · 2026-11-06T16:36Z

Delta review against the FAIL at head 14487c5 (comment 6689544173). The delta is a merge of main plus one commit answering M1 (eval case 5, claim widened on the card), S1 and S2, plus the PR body. The seat review runs at CONTRACT_REVIEW_TIER in an isolated subagent.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:skills#3 · huangyiirene · session_01J8VVAj7CvjkGXvgXzd6RZ7

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 9a449ed96a21307612ff086b0a18990e56dc637d
Local-runs: none
Scope: skills/objectstack-api/SKILL.md, skills/objectstack-api/evals/endpoints-auth-routes.json + .changeset/api-skill-delete-recycle-bin.md, packages/objectql/src/recycle-bin/capture.ts, packages/objectql/src/recycle-bin/retention.ts, packages/objectql/src/engine.ts (runByIdDelete), packages/metadata-protocol/src/protocol-data-writes.ts (deleteData), packages/platform-objects/src/system/sys-recycle-bin.object.ts, packages/spec/src/data/recycle-bin.zod.ts, packages/spec/src/data/api-derivation.ts, packages/spec/src/data/object.zod.ts (lifecycle class rules), packages/spec/src/security/permission.zod.ts, packages/spec/src/system/constants/platform-object-names.ts, packages/plugins/plugin-security/src/permission-evaluator.ts, docs/adr/0148-recycle-bin-separate-store-restore-under-original-ids-retention-in-days.md, packages/skills/scripts/sync-catalog.mjs, ui/scripts/skill-bundle-root.mjs

This is a delta review. The previous record, on 14487c5027728964e39e56ee0cec56bff9c03d2a, was a FAIL: must-fix M1 and should-fixes S1 and S2.

Inputs:

The merge. It brings in only main's own two commits, #30825 and #30861. Neither touches a file in this PR or any cited file: git diff --stat 000a4c0d3 c77dd17f over every path in Scope: is empty. So the cited code read in the previous record is unchanged. The net diff against main is exactly these three files:

  • .changeset/api-skill-delete-recycle-bin.md: +8
  • skills/objectstack-api/SKILL.md: +7 −7
  • skills/objectstack-api/evals/endpoints-auth-routes.json: +2 −2, case 5 only

Check-runs on 9a449ed9. All 45 runs are complete, and none is in progress. Every non-skipped run is success, including:

  • Test Core (1/8), Test Core (2/8) and Test Core
  • Lint & Repo Gates and every Lint · lane
  • every Type Check · lane, and TypeScript Type Check
  • Build Core, Check Changeset, Governed Surface Queue Guard and PR-body guards
  • Test UI, Live E2E UI and Dogfood Regression Gate

① Derived judgments

There are still no accept-set changes. No schema, no ApiMethod / ApiOperation enum and no API_METHOD_DERIVATION row changed; restore and purge stay at flag: () => false. No permission key and no route changed.

The published-surface change is the text of the @objectstack/skills catalog, where sync-catalog.mjs ships all of skills/**. Each change is right on main:

  • L48, route comment: # Delete record (business-object rows → recycle bin). Right. The REST door's deleteData sends where: { id } with no multi, which reaches runByIdDelete. recycleBinTakesObject takes only names outside sys_, cloud_ and ai_. A platform object served on the same route is now correctly left out of the comment.
  • L271–272: unchanged from the previous head. It is still right.
  • L411–414, pitfall 2: right on every clause; see ④.
  • Eval case 5: now right; see ④ for M1.

② Semver level

'@objectstack/skills': patch still matches. The delta adds one eval fixture in the same package. That is catalog text, with no capability added or removed.

Clause-②: no is right: there is no published schema, no packages/spec surface and no governed spec path in the diff. Metadata-protocol: none is right. The changeset prose now names both the eval change and the "nothing lists or restores it over the API yet" teaching, and it matches the tree. The PR body and changeset carry the same three lines, and Check Changeset is green.

③ Boundary flags

  • Claim surface. The seat widened it to eval case 5 only: expected_output plus the trackHistory / lifecycle tokens of must_contain, keeping 'restore'. The diff stays inside that. Lines 44–53 of the eval change in place, and no other case moved. The previous record's escalation is resolved.
  • Partial scope, Refs #30847. Still right. engine: snapshot deletes into the recycle bin, restore under original ids, purge, and the TTL reap (#3146 slice 3) #30237 is still open (checked), and the tree names no restore or purge door, grant key or route. The card must stay open with Blocked-by: #30237.
  • Contract-review tier waiver. It is served by this record at CONTRACT_REVIEW_TIER.
  • Dropped trackHistory / lifecycle pointer. The previous judgment stands: no rule that is still true was lost. The eval no longer grades for those words, so the side effect behind M1 is closed.
  • Tier H approval for skills/** is still owed by a governed approver. That is outside this record.
  • The PR body's budget and Verification account were not re-run (read-only). The head's check-runs are the verdicts.

④ Design judgment

skills/objectstack-api/SKILL.md at the head, judged against main and against eval case 5, the bundle's scenario for this topic.

M1: resolved. Eval case 5 at the head:

  • expected_output is true on main. It says restore / purge never derive. It says a by-id delete of a business object already keeps its rows in sys_recycle_bin, but nothing lists or restores them over the API yet, so there is no restore operation to add and no custom soft-delete to build. Each clause holds:

    • flag: () => false.
    • capture in runByIdDelete, with recycleBinTakesObject limiting it to business objects.
    • sys_recycle_bin is apiEnabled: false, apiMethods: [].
    • there is no engine restore or REST door; restore and purge are in DESTRUCTIVE_OPERATIONS.
    • ADR-0148 D1.
  • must_contain is ["apiMethods", "/batch", "sys_recycle_bin", "recycle bin"]. Every token is taught by the corrected bundle markdown:

    • sys_recycle_bin at L412.
    • recycle bin at L48, L272 and L412.
    • apiMethods and /batch as before.

    Multi-word tokens are an established shape across the catalog's evals (99 of them), so recycle bin is gradable.

  • must_not_contain still holds 'restore' and 'search'.

  • The file parses as JSON.

  • An agent answering prompt 5 from the corrected SKILL.md now satisfies the eval, not the reverse. The phrases "DELETE is a hard delete", "trash surface" and "no recycle bin" no longer appear anywhere under skills/**.

S1: resolved, and still true. See ①. The comment now limits the bin to business-object rows on a by-id route. That is the exact reach of openRecycleBinCapture.

S2: resolved, and still true. "Nothing lists or restores it over the API yet" matches sys_recycle_bin's apiEnabled: false / apiMethods: [] and the absence of any restore or purge door. An agent is no longer pointed at a GET /api/v1/data/sys_recycle_bin that the data API refuses.

Pitfall 2, re-read whole. "Treating DELETE as final or as restorable. A by-id delete keeps a business object's rows in the recycle bin (sys_recycle_bin, 30 days by default; not multi: true deletes, platform rows or rows deleted before the bin existed); nothing lists or restores it over the API yet. No soft-delete." Every clause holds against the code read in the previous record, none of which moved:

  • retention resolver → env → 30, stamped as purge_after.
  • the predicate refusal for multi: true.
  • hasPlatformObjectPrefix.
  • ADR-0148 D4, forward-only.

No undelivered door, grant or route is taught.

L271–272 still reads correctly inside the derived-verb table. "Declaring any derived verb in apiMethods is stripped at parse" still governs restore and purge.

Findings: none. No must-fix and no should-fix remains.

One note, not a finding. "Over the API" is narrower than the truth on main: no engine-level restore exists either. Nothing the sentence states is false, and an agent reading it is not sent to a door that exists elsewhere. The note is recorded only so that the #30237 follow-up rewrites this sentence when the doors land.

Implemented-by: claude/project-thread-zyyxri
Reviewed-by: session_01J8VVAj7CvjkGXvgXzd6RZ7

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:skills#3 · huangyiirene · session_01J8VVAj7CvjkGXvgXzd6RZ7

维护者速读

席位核对 — ACCEPT · domain:skills#3 · head 9a449ed96a21307612ff086b0a18990e56dc637d

  1. 形态: draft → main,Refs #30847;'@objectstack/skills': patch,Clause-②: no,Metadata-protocol: none;作者 objectstack-fleet[bot];3 个文件,在认领面及席位加宽(eval 第 5 例)之内。
  2. 前提实测: 增量复核读 main 代码确认:按 id 删除走 runByIdDelete → 回收站快照;sys_recycle_bin 为 apiEnabled: false、apiMethods: [],无 restore / purge 入口;M1(eval 第 5 例旧说法)、S1(L48 只说业务对象行)、S2(「over the API」)均已闭。
  3. 门禁(钉 head 9a449ed9): dev 派生门禁 45 条全部 exit 0;check-runs 45 条全部完成,非 skip 者全 success(含 Test Core 两片)。check-governed-merges:Tier H(skills/**)。
  4. 接受的偏差: PR 正文速读的行号(L268–270、L417–420)是早先 head 的读数,以本评论 L272–273、L411–414 为准;不影响内容。
  5. 范围外去向: restore / purge 入口、权限与 REST 路由的教学留在 skills: objectstack-api still teaches that DELETE is unrecoverable and that there is no recycle bin (#30237, ADR-0148) #30847,合并后该卡以 Blocked-by: #30237 回队。Skills-docs: self。
  6. 落地条款: Tier H。保持 draft,挂 pm:awaiting-merge,请 os-zhuang、hotlong 审批;获批后本席过落地前检、清标、ready、auto-merge;维护者直合亦可。

@objectstack-fleet objectstack-fleet Bot added the pm:awaiting-merge PR only: landing waits on the maintainer's own review and merge (SKILL.md 复核 ②); never on an issue label Nov 6, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review November 6, 2026 16:59
@os-zhuang
os-zhuang added this pull request to the merge queue Nov 6, 2026
@objectstack-fleet objectstack-fleet Bot removed the pm:awaiting-merge PR only: landing waits on the maintainer's own review and merge (SKILL.md 复核 ②); never on an issue label Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 42259097933 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

分类:failure —— 按下面的日志分诊。

失败的 job(日志抽取,best effort):

  • Dogfood Regression Gate (3/3) — 失败步骤: Boot example apps and exercise real user flows

    @objectstack/dogfood:test:  FAIL   isolated  test/admin-platform-admin-standing.dogfood.test.ts > #9482: what an ObjectStack platform admin gets from every /admin/ route > no /admin/ route refuses the
      ↳ 失败原因: @objectstack/dogfood:test: AssertionError: the stack serves /admin/ route(s) this file neither admits, refuses-by-design, nor classifies as answering for a non-authorization reason. A new admin route 
    

↳ 失败原因 是判读的关键:超时(Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言(AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError。 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 16 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:skills#3 · huangyiirene · session_01J8VVAj7CvjkGXvgXzd6RZ7

Dequeued, not this PR's failure. Queue build 42259097933 failed test/admin-platform-admin-standing.dogfood.test.ts (an unclassified /admin/ route). This PR changes only skill text and one eval fixture. #31012 records the stack: its root is #30991, which adds an admin door and is itself out of the queue, unmerged, and this PR inherited that tree. main (69e133ea0) does not carry #30991.

The ACCEPT (6690169452) and the PASS on this head (6690135496) still stand. No approval is on record yet, so the seat does not re-arm this PR itself: it needs one approval from os-zhuang or hotlong, or a maintainer can queue it again. pm:awaiting-merge is back on.

@objectstack-fleet objectstack-fleet Bot added the pm:awaiting-merge PR only: landing waits on the maintainer's own review and merge (SKILL.md 复核 ②); never on an issue label Nov 6, 2026
Merged via the queue into main with commit 0306761 Nov 6, 2026
59 checks passed
@os-zhuang
os-zhuang deleted the claude/project-thread-zyyxri branch November 6, 2026 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation pm:awaiting-merge PR only: landing waits on the maintainer's own review and merge (SKILL.md 复核 ②); never on an issue size/s tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants